Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect Snowflake Quoting #930

Closed
wants to merge 2 commits into from
Closed

Conversation

ludwig-solita
Copy link

Problem

Quoting isn't respected when altering Snowflake tables with non-ansi-sql compliant names. This throws an error when syncing columns in incremental models. I didn't create a new issue since this already seems to recognize the problem
dbt-labs/dbt-adapters#250

I created a new PR since I messed up the commit history with an unverified Github Account. Original PR: #923
@VersusFacit

image
image
image

Solution

Instead of just using column.name for the alter statements I've added the adapter.quote to properly respect the quoting configuration in yaml-files

Tests

I haven't written any tests in this project before so any help would be appreciated. The below code would essentially be the test.

Configure project.yml to use quoting for identifiers

quoting:
  identifier: true

Create the table with CTAS

{{
    config(
        materialized="incremental",
        on_schema_change='sync_all_columns'
    )
}}
SELECT 'Hello world!' as original_column

Add new columns

{{
    config(
        materialized="incremental",
        on_schema_change='sync_all_columns'
    )
}}
SELECT 'Hello world!' as original_column
,'Greetings. I come from a non-ansi compliant planet' as "NoN AnSi CoMpLiAnT CoLuMn"
,'Greetings. I come from an ansi compliant planet' as ANSI_COMPLIANT_COLUMN

Checklist

  • I have read the contributing guide and understand what's expected of me

  • I have run this code in development and it appears to resolve the stated issue
    image

  • This PR includes tests, or tests are not required/relevant for this PR
    It's a quite minor change.

  • This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX
    It does affect the adapter. I've had discussions with @ernestoongaro regarding the problem.

@cla-bot cla-bot bot added the cla:yes label Mar 15, 2024
@dataders
Copy link
Contributor

dataders commented Mar 21, 2024

@ludwig-solita is there an open dbt-snowflake issue for this? If not can you please open one? thanks! I couldn't find one on dbt-labs/dbt-core#923 either

@ludwig-solita
Copy link
Author

Quoting isn't respected when altering Snowflake tables with non-ansi-sql compliant names. This throws an error when syncing columns in incremental models. I didn't create a new issue since this already seems to recognize the problem

Found this one in the core repo. dbt-labs/dbt-adapters#250

Copy link
Contributor

This PR has been marked as Stale because it has been open with no activity as of late. If you would like the PR to remain open, please comment on the PR or else it will be closed in 7 days.

@github-actions github-actions bot added the Stale label Dec 23, 2024
Copy link
Contributor

Although we are closing this PR as stale, it can still be reopened to continue development. Just add a comment to notify the maintainers.

@github-actions github-actions bot closed this Dec 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants